[local-explorer-ui][miniflare] Improve resending of emails from local explorer - #15567
[local-explorer-ui][miniflare] Improve resending of emails from local explorer #15567tpmmorris wants to merge 11 commits into
Conversation
🦋 Changeset detectedLatest commit: 2d5b2ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
penalosa
left a comment
penalosa
left a comment
There was a problem hiding this comment.
This is a very large PR, which makes it quite hard to review. I wonder if you'd be able to either:
- split it up into more reviewable chunks
- go through and do a review yourself, pointing out the bits that you think need special attention from reviewers/explaining the decisions you made along the way
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
tpmmorris
left a comment
tpmmorris
left a comment
There was a problem hiding this comment.
This PR is used to update local explorer's email group to make resend row-based, looking up emails in storage to resend the stored data. Implementation ensures local explorer instances running peers on older versions of miniflare can still operate.
Generated files and tests consume approx. +2200/-200 of the diff.
The primary implementation areas are:
- storage identity (capture ID introduction)
- server-side resend
- UI action availability + lifecycle
| emptyState: ReactNode; | ||
| error: string | null; | ||
| getRow: (item: T) => EmailListRow; | ||
| getRow: (item: T, index: number) => EmailListRow; |
There was a problem hiding this comment.
Index is used here to allow a local explorer exposing peers, possibly with older versions of miniflare, to unambiguously render rows when looking up via message id, as lookup is now primarily done with the guaranteed unique capture ID. Peers using an older version of miniflare that does not have capture ID can be inspected, but can't be resent (as of e4fd618).
| let angleDepth = 0; | ||
| let commentDepth = 0; | ||
| let escaped = false; | ||
| let groupDepth = 0; |
There was a problem hiding this comment.
Tracks whether the parser is inside of an email group to parse it correctly, groups are written as:
ExampleGroup: name@domain.com, anotherName@domain.com;
| interface RoutingEmailActions { | ||
| dialogDraft?: TestEmailDraft; | ||
| dialogOpen: boolean; | ||
| dialogWorker?: string; | ||
| editAndResend: (email: RoutingEmail) => Promise<void>; | ||
| getRowActionState: (email: RoutingEmail) => EmailRoutingActionState; | ||
| handleDialogOpenChange: (open: boolean) => void; | ||
| openBlankComposer: () => void; | ||
| requestInboxRefresh: (expectedGeneration: number) => void; | ||
| resend: (email: RoutingEmail) => Promise<void>; | ||
| workerGeneration: number; | ||
| } |
There was a problem hiding this comment.
Row actions depend on the emails result, if an email is truncated/not sent from the composer, editAndResend is unavailable, but direct resend is.
| let lookup: ReceivedCaptureOperationLookup; | ||
| try { | ||
| lookup = await loadReceivedCaptureForOperation(c, worker, captureId); |
There was a problem hiding this comment.
Resending is server side, looking up a capture in DO storage by its capture ID and worker, and creating a new send with the stored data. Truncated emails are resent as they are stored, so behaviour may not be identical. In the future we can possibly use local file capture for all email storage, making this redundant, but this is out of scope for now.
| captureTruncated: z.boolean().optional(), | ||
| capturedPortion: z.boolean().optional(), |
There was a problem hiding this comment.
captureTruncated describes whether the email was truncated when captured. capturedPortion is inherited, and describes whether this email was truncated at any point in its history, such as a sent email that was truncated and then resent; the resend wasn't directly truncated, so captureTruncated = false, but the original email it was resent using was truncated, so capturedPortion = true.
| }; | ||
|
|
||
| export interface EmailCaptureContext { | ||
| origin?: EmailCaptureOrigin; |
There was a problem hiding this comment.
Only composer constructed emails are permitted to be edited and resent. If the email is marked unknown it may contain fields not supported by the composer, so shouldn't be edited in the composer.
| bodyRawBase64: string[], | ||
| createCaptureId: () => string = () => crypto.randomUUID() | ||
| ): Promise<string> { | ||
| for (let attempt = 0; attempt < 3; attempt++) { |
There was a problem hiding this comment.
Attempt up to 3 ID generations. Failure to find a non-colliding UUID here will cause a capture failure, but is practically impossible.
| let mime: Uint8Array; | ||
| let messageId: string; | ||
| try { | ||
| const source = base64ToBytes(email.rawBase64); | ||
| messageId = synthesizeMessageId(extractAddressFromString(email.from)); | ||
| mime = setMessageIdHeader(source, messageId); |
There was a problem hiding this comment.
Injects a new message ID for resent messages to simulate a "new" send occurrence.
| storeSent(email: StoredSendingEmail): void { | ||
| this.#insert( | ||
| "sent", | ||
| messageIdToStorageId(email.messageId), |
There was a problem hiding this comment.
Sending emails still uses messageId for lookup, as this is uniquely synthesised per send. Routing emails now use a capture ID which is generated on capture. This is because in production, message IDs are unique per send, not per receipt, an email delivered to multiple recipients will have the same message ID for each of them, and it is reasonable to adopt similar behaviour here.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
3 participants
Updates Emails in Local Explorer
Previously, resending always sent the last successful email without the option to edit the email before sending. The new resending is row-by-row, with buttons available to 'edit and resend' or 'resend immediately' on any captured email. Emails can only be edited if they are provided by the 'Send Test Email' composer, rather than sent via the
/cdn-cgi/local/emailendpoint, which may have fields which can't be parsed to the composer.This allows users to quickly amend emails, or send the same email to an adjusted worker.